Area of preview on hover should be shrink to the size of the text displayed #13555

xIshanSandhux · 2025-07-18T21:52:10Z

The issue was basically the box height was not getting set based on the contents of the entry. The default height was always being used.

So currently I have updated the PreivewViewer.java file to get the current height based on the content of the entry. Initially I did not add any extra padding but then the last bit of the content was getting cut off. So as of now i have added extra 10 and it gives me the last bit content properly and lil extra room. I can reduce that if you want.

I have fixed 70% of the issue.
However still on the first render the old tooltip box height and width is being displayed but after that I am getting the correct box height and width based on the text. So after figuring out how to fix that I will change the status of the PR to "ready for review".

Current look with extra 10 padding (both height and width:

Steps to test

Mandatory checks

I own the copyright of the code submitted and I license it under the MIT license
Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
[/] Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (if change is visible to the user)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

jabgui/src/main/java/org/jabref/gui/preview/PreviewViewer.java

koppor

@xIshanSandhux Ever tried this?

There should not be any horizontal scroll bar

Could be wider

White area should not be shown

xIshanSandhux · 2025-08-02T17:43:53Z

Applied the suggestion, but i did try this before, and i was getting the same result so..

currently the issue is that with each hover the width keeps changing so I am trying to figure out why and where is that happening.

fixing height to not have scroll bar

xIshanSandhux · 2025-08-02T19:41:30Z

jabgui/src/main/java/org/jabref/gui/preview/PreviewViewer.java

@@ -125,6 +125,16 @@ private void configurePreviewView(ThemeManager themeManager) {
                return;
            }

+            if (previewView.getEngine().executeScript("document.getElementById('content').scrollHeight") instanceof java.lang.Number heightVal) {
+                double actualHeight = (heightVal).doubleValue();


xIshanSandhux · 2025-08-02T19:41:39Z

jabgui/src/main/java/org/jabref/gui/preview/PreviewViewer.java

+            }
+
+            if (previewView.getEngine().executeScript("document.getElementById('content').scrollWidth") instanceof java.lang.Number widthVal) {
+                double actualWidth = (widthVal).doubleValue();


trag-bot · 2025-08-02T21:18:46Z

@trag-bot didn't find any issues in the code! ✅✨

jabref-machine · 2025-08-02T21:25:00Z

Your code currently does not meet JabRef's code guidelines. We use OpenRewrite to ensure "modern" Java coding practices. You can see which checks are failing by locating the box "Some checks were not successful" on the pull request page. To see the test output, locate "Tests / OpenRewrite (pull_request)" and click on it.

The issues found can be automatically fixed. Please execute the gradle task rewriteRun from the rewrite group of the Gradle Tool window in IntelliJ, then check the results, commit, and push.

xIshanSandhux · 2025-08-02T21:29:46Z

@koppor could you please check it out and let me know if it works now.

This is what I am getting now. I have tested using the entries provided in the example library.

If it works, I will update the PR so that its compliant Jabref's code guidelines and will update the status to "ready for review".
If it does not work, then let me know what is not working, and I will fix it accordingly

koppor

Its OKish now.

Only at the first call, it looks as follows

Second call

koppor · 2025-08-03T13:45:04Z

jabgui/src/main/java/org/jabref/gui/preview/PreviewViewer.java

+            }
+            previewView.getEngine().executeScript("document.body.style.overflow='hidden';");
+
+            if (previewView.getEngine().executeScript("document.getElementById('content').scrollWidth") instanceof java.lang.Number widthVal && !widthSet.get()) {


Please comment why you need widthSet and not heightSet

koppor · 2025-08-03T13:45:52Z

jabgui/src/main/java/org/jabref/gui/preview/PreviewViewer.java

@@ -87,6 +87,7 @@ function getSelectionHtml() {
    private @Nullable BibEntry entry;
    private PreviewLayout layout;
    private String layoutText;
+    private final AtomicBoolean widthSet = new AtomicBoolean(false);


Does one really need an AtomicBoolean here? If the method is executed concurrently multiple times, there is a bigger flaw in the architecture.

looked more into it and its not required. using normal bool now

koppor · 2025-08-03T13:46:13Z

jabgui/src/main/java/org/jabref/gui/preview/PreviewViewer.java

@@ -52,7 +53,6 @@
 public class PreviewViewer extends ScrollPane implements InvalidationListener {

    private static final Logger LOGGER = LoggerFactory.getLogger(PreviewViewer.class);
-


Please keep the empty line as the thing which follow these multiple lines

trag-bot · 2025-08-03T14:07:18Z

@trag-bot didn't find any issues in the code! ✅✨

Yubo-Cao · 2025-08-06T00:35:02Z

I tried this PR. On the first render (i.e., just open JabRef first time and open the example library and hover the entry to see the preview for the first time), the height grows to the full screen area.

Yubo-Cao

A bug is present, and some alternative approaches should be encouraged. I am pretty sure the EntryEditor's preview also got introduced a bug as a results of this since, if you first preview a very small entry then go to a big one, this new implementation will unnaturally fold the preview (as it always records the first width of the first entry viewed as the width of all subsequent preview).

Yubo-Cao · 2025-08-06T01:03:15Z

jabgui/src/main/java/org/jabref/gui/preview/PreviewViewer.java

@@ -125,6 +126,25 @@ private void configurePreviewView(ThemeManager themeManager) {
                return;
            }

+            previewView.getEngine().executeScript("document.body.offsetHeight;");


Why are we looking at this? If the purpose is to force the webview layout the element, write out such intent (and say maybe there is a case that such forcing is necessary).

Yubo-Cao · 2025-08-06T01:10:33Z

jabgui/src/main/java/org/jabref/gui/preview/PreviewViewer.java

+            }
+            previewView.getEngine().executeScript("document.body.style.overflow='hidden';");
+
+            if (previewView.getEngine().executeScript("document.getElementById('content').scrollWidth") instanceof java.lang.Number widthVal && !widthSet.get()) {


Note that Previewer is only created once in the Tooltip and here after, the same instance of the Previewer is created. In that case, we are just being arbitrary to say, however wide the first entry it is, let's just it's width. And granted, what you are facing right now is basically impossible. You cannot possibly get the scroll height of an element without knowing/first confining to some set width.

With that, I felt maybe you could hardcode some width value for the MainTableToolTip use case rather than create the appearance of respecting element's natural width. Or, to handle cases with tooltip element that's actually just less than the predetermined width, maybe a bit of JS is needed, instead of Java :)

xIshanSandhux added 2 commits July 18, 2025 14:17

Initial fix

202af9c

Update MainTableTooltip.java

eda59fb

Siedlerchr reviewed Jul 19, 2025

View reviewed changes

jabgui/src/main/java/org/jabref/gui/preview/PreviewViewer.java Outdated Show resolved Hide resolved

calixtus changed the title ~~Fix for issue 12351: Area of preview on hover should be shrink to the size of the text displayed~~ Area of preview on hover should be shrink to the size of the text displayed Jul 19, 2025

xIshanSandhux and others added 2 commits July 19, 2025 13:15

got the width based on the text

6d6fd96

Apply suggestion from code review

a24adc8

koppor requested changes Jul 28, 2025

View reviewed changes

fixed width for the hover

875b1b2

fixing height to not have scroll bar

xIshanSandhux commented Aug 2, 2025

View reviewed changes

fixed height and hid the scroll bars

2d4ff2a

koppor requested changes Aug 3, 2025

View reviewed changes

subhramit marked this pull request as ready for review August 3, 2025 14:05

subhramit requested a review from Yubo-Cao August 3, 2025 14:09

Yubo-Cao requested changes Aug 6, 2025

View reviewed changes

		@@ -52,7 +53,6 @@
		public class PreviewViewer extends ScrollPane implements InvalidationListener {

		private static final Logger LOGGER = LoggerFactory.getLogger(PreviewViewer.class);

Uh oh!

Area of preview on hover should be shrink to the size of the text displayed #13555

Are you sure you want to change the base?

Area of preview on hover should be shrink to the size of the text displayed #13555

Conversation

xIshanSandhux commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current look with extra 10 padding (both height and width:

Steps to test

Mandatory checks

Uh oh!

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

xIshanSandhux commented Aug 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trag-bot bot commented Aug 2, 2025

Uh oh!

jabref-machine commented Aug 2, 2025

Uh oh!

xIshanSandhux commented Aug 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trag-bot bot commented Aug 3, 2025

Uh oh!

Yubo-Cao commented Aug 6, 2025

Uh oh!

Yubo-Cao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xIshanSandhux commented Jul 18, 2025 •

edited

Loading

xIshanSandhux commented Aug 2, 2025 •

edited

Loading